[UR][Benchmarks] Support for Unitrace for all benchmarks #19320

mateuszpn · 2025-07-07T09:03:57Z

Run main.py script with --unitrace to get unitrace logs for every benchmark.
Use --unitrace-inclusive to get benchmark results and unitrace logs.

Signed-off-by: Mateusz P. Nowak <mateusz.p.nowak@intel.com>

Copilot

Pull Request Overview

Adds support for Unitrace tracing in all benchmarks by introducing new CLI flags, integrating Unitrace setup/teardown logic, and extending benchmark run methods.

Introduce --unitrace and --unitrace-inclusive flags and related options
Add unitrace.py utilities (prepare, cleanup, handle, prune)
Update run_bench and individual benchmarks to accept unitrace_timestamp and skip exclusions

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
devops/scripts/benchmarks/utils/unitrace.py	New module for Unitrace log handling
devops/scripts/benchmarks/options.py	Added CLI options for Unitrace and `save_name`
devops/scripts/benchmarks/main.py	Integrated Unitrace flags into workflow and CLI
devops/scripts/benchmarks/html/data.js	Removed trailing whitespace
devops/scripts/benchmarks/history.py	Updated `save` signature to include `timestamp`
devops/scripts/benchmarks/benches/velocity.py	Extended `run` to forward `unitrace_timestamp`
devops/scripts/benchmarks/benches/umf.py	Extended `run` to forward `unitrace_timestamp`
devops/scripts/benchmarks/benches/test.py	Extended `run` to forward `unitrace_timestamp`
devops/scripts/benchmarks/benches/syclbench.py	Extended `run` to forward `unitrace_timestamp`
devops/scripts/benchmarks/benches/llamacpp.py	Extended `run` to forward `unitrace_timestamp`
devops/scripts/benchmarks/benches/gromacs.py	Extended `run` to forward `unitrace_timestamp`
devops/scripts/benchmarks/benches/compute.py	Added Unitrace exclusion list and timestamp logic
devops/scripts/benchmarks/benches/benchdnn_list.py	Defined `unitrace_exclusion_list` for oneDNN bench
devops/scripts/benchmarks/benches/benchdnn.py	Integrated Unitrace options for oneDNN bench
devops/scripts/benchmarks/benches/base.py	Updated `run_bench` signature to add Unitrace hooks

Comments suppressed due to low confidence (2)

devops/scripts/benchmarks/utils/unitrace.py:26

[nitpick] Consider adding unit tests for prune_unitrace_dirs (and other helper functions) to verify that old directories are removed correctly and edge cases are handled.

def prune_unitrace_dirs(base_dir, FILECNT=10):

devops/scripts/benchmarks/benches/compute.py:229

[nitpick] This exclusion list is duplicated in benchdnn_list.py; consider centralizing it in one module to avoid inconsistencies.

    unitrace_exclusion_list = [

Copilot · 2025-07-07T09:20:02Z

devops/scripts/benchmarks/main.py

 from utils.validate import Validate
 from utils.detect_versions import DetectVersion
 from presets import enabled_suites, presets
+from utils.oneapi import get_oneapi


The import get_oneapi is not used in this file; consider removing it to avoid unused imports.

Suggested change

from utils.oneapi import get_oneapi

Copilot · 2025-07-07T09:20:03Z

devops/scripts/benchmarks/utils/unitrace.py

+    bench_dir = f"{options.unitrace_res_dir}/{options.save_name}_{unitrace_timestamp}"
+    os.makedirs(bench_dir, exist_ok=True)
+
+    unitrace_output = f"{bench_dir}/{name}_{unitrace_timestamp}"


[nitpick] Constructing file paths with hardcoded / can break on some platforms; consider using os.path.join(options.unitrace_res_dir, f"{options.save_name}_{unitrace_timestamp}") for portability.

Suggested change

bench_dir = f"{options.unitrace_res_dir}/{options.save_name}_{unitrace_timestamp}"

os.makedirs(bench_dir, exist_ok=True)

unitrace_output = f"{bench_dir}/{name}_{unitrace_timestamp}"

bench_dir = os.path.join(options.unitrace_res_dir, f"{options.save_name}_{unitrace_timestamp}")

os.makedirs(bench_dir, exist_ok=True)

unitrace_output = os.path.join(bench_dir, f"{name}_{unitrace_timestamp}")

Copilot · 2025-07-07T09:20:03Z

devops/scripts/benchmarks/main.py

+    parser.add_argument(
+        "--unitrace",
+        action="store_true",
+        help="Unitrace tracing for sigle iteration of benchmarks",


Typo in help text: "sigle" should be "single".

Suggested change

help="Unitrace tracing for sigle iteration of benchmarks",

help="Unitrace tracing for single iteration of benchmarks",

Copilot · 2025-07-07T09:20:04Z

devops/scripts/benchmarks/benches/base.py

@@ -86,7 +87,14 @@ def get_adapter_full_path():
        ), f"could not find adapter file {adapter_path} (and in similar lib paths)"

    def run_bench(


[nitpick] Avoid mutable default arguments like extra_unitrace_opt=[]; use None and set to an empty list inside the function to prevent unintended sharing.

pbalcer · 2025-07-07T09:07:11Z

devops/scripts/benchmarks/utils/unitrace.py

+                "-DBUILD_WITH_XPTI=1",
+                "-DBUILD_WITH_MPI=0",
+            ],
+            ld_library=get_oneapi().ld_libraries() + [f"{options.sycl}/lib"],


🤔 removed

pbalcer · 2025-07-07T09:08:11Z

devops/scripts/benchmarks/utils/unitrace.py

@@ -0,0 +1,182 @@
+# Copyright (C) 2024-2025 Intel Corporation


pbalcer · 2025-07-07T09:09:15Z

devops/scripts/benchmarks/utils/unitrace.py

+        print(f"Moved {pid_json_files[0]} to {dst}")
+
+    # Prune old unitrace directories
+    prune_unitrace_dirs(options.unitrace_res_dir, FILECNT=5)


You use this only here. Why bother with defining a default value of an argument that you are going to override anyway?

Debugging leftover, removed

pbalcer · 2025-07-07T09:12:34Z

devops/scripts/benchmarks/utils/unitrace.py

+
+    shutil.move(os.path.join(options.benchmark_cwd, pid_json_files[0]), dst)
+    if options.verbose:
+        print(f"Moved {pid_json_files[0]} to {dst}")


all the logs will need to be changed after #19158 is merged.

pbalcer · 2025-07-07T09:35:44Z

devops/scripts/benchmarks/utils/unitrace.py

+    return bench_dir, unitrace_output, unitrace_command
+
+
+def handle_unitrace_output(bench_dir, unitrace_output, timestamp):


I feel like this is all very complicated. Maybe we can use directory structure to help us?

Let's say we have two benchmarks: "SubmitKernel" and "Hashtable". We can organize the unitrace output such that each trace file is in its own directory:

.../results/traces/SubmitKernel/YYYYMMDD_HHMMSS_[name].PID.out .../results/traces/Hashtable/YYYYMMDD_HHMMSS_[name].PID.out

Removing old results would become simply removing all but 5 newest files in a directory. We wouldn't have to find, rename and move files, since we could make unitrace output them to the destination place upfront.

Done, various approaches are possible, no problem to follow yours.

pbalcer · 2025-07-07T09:37:30Z

devops/scripts/benchmarks/main.py

+    parser.add_argument(
+        "--unitrace",
+        action="store_true",
+        help="Unitrace tracing for sigle iteration of benchmarks",
+    )
+
+    parser.add_argument(
+        "--unitrace-inclusive",
+        action="store_true",
+        help="Regular run of benchmarks iterations and unitrace tracing in single additional run",
+    )


please do --unitrace inclusive and --unitrace as a single option, not two separate ones. See how --output-html is done.

pbalcer · 2025-07-07T09:43:25Z

devops/scripts/benchmarks/benches/compute.py

@@ -224,6 +224,71 @@ def parse_unit_type(compute_unit):


 class ComputeBenchmark(Benchmark):
+
+    # list of benchmarks to exclude from unitrace due to SIGSEGV, SIGABRT or timeouts
+    unitrace_exclusion_list = [


We should just let things fail. If unitrace is failing, it's an urgent bug that needs to be fixed.

Signed-off-by: Mateusz P. Nowak <mateusz.p.nowak@intel.com>

devops/scripts/benchmarks/utils/unitrace.py

devops/scripts/benchmarks/main.py

devops/scripts/benchmarks/history.py

devops/scripts/benchmarks/benches/base.py

devops/scripts/benchmarks/benches/benchdnn.py

devops/scripts/benchmarks/utils/unitrace.py

PatKamin · 2025-07-07T12:21:54Z

devops/scripts/benchmarks/utils/unitrace.py

+                    print(f"Renamed {src} to {dst}")
+                break
+
+    # Handle {name}.{pid}.json files in cwd: move and rename to {self.name()}_{timestamp}.json


Doesn't unitrace already have an option for changing the default name of jsons and .pid logs?

unitrace produces two kinds of output files - traces, configurable with --output (pid is added anyway), and jsons for visualization in Perfetto UI, created with no control over their names - just ..json in cwd

Bummer. Maybe we could change unitrace to use the output name for both outputs. The visualization json could be something like name.pid.vis.json.

devops/scripts/benchmarks/utils/unitrace.py

pbalcer

One general comment - when under tracing, we should reduce the number of iterations etc, to shorten the duration of the benchmark.
If we really need to disable a benchmark (because it's difficult to make it sufficiently short for example), the Benchmark class should have something like traceable() to filter out which benchmarks should / shouldn't be traced.

mateuszpn · 2025-07-08T12:22:41Z

the Benchmark class should have something like traceable() to filter out which benchmarks should / shouldn't be traced.

Added in Benchmark, and a stub in ComputeBenchmark

Signed-off-by: Mateusz P. Nowak <mateusz.p.nowak@intel.com>

mateuszpn added 4 commits June 30, 2025 11:04

Add Unitrace support

4d32fe5

Signed-off-by: Mateusz P. Nowak <mateusz.p.nowak@intel.com>

Merge branch 'intel:sycl' into unitrace

72d8108

Add Unitrace support

d653883

Signed-off-by: Mateusz P. Nowak <mateusz.p.nowak@intel.com>

Merge branch 'unitrace' of github.com:mateuszpn/llvm into unitrace

5834aa9

mateuszpn requested a review from a team as a code owner July 7, 2025 09:03

mateuszpn temporarily deployed to WindowsCILock July 7, 2025 09:04 — with GitHub Actions Inactive

mateuszpn added 2 commits July 7, 2025 09:12

Add list of Unitrace exclusions

d8f6b2b

Signed-off-by: Mateusz P. Nowak <mateusz.p.nowak@intel.com>

Merge remote-tracking branch 'upstream/sycl' into unitrace

01c1c1a

mateuszpn requested a review from Copilot July 7, 2025 09:17

Copilot AI reviewed Jul 7, 2025

View reviewed changes

mateuszpn temporarily deployed to WindowsCILock July 7, 2025 09:25 — with GitHub Actions Inactive

pbalcer reviewed Jul 7, 2025

View reviewed changes

Final fixes

be36447

Signed-off-by: Mateusz P. Nowak <mateusz.p.nowak@intel.com>

PatKamin reviewed Jul 7, 2025

View reviewed changes

devops/scripts/benchmarks/benches/benchdnn.py Outdated Show resolved Hide resolved

devops/scripts/benchmarks/utils/unitrace.py Outdated Show resolved Hide resolved

devops/scripts/benchmarks/utils/unitrace.py Outdated Show resolved Hide resolved

PatKamin reviewed Jul 7, 2025

View reviewed changes

pbalcer reviewed Jul 7, 2025

View reviewed changes

mateuszpn temporarily deployed to WindowsCILock July 8, 2025 11:47 — with GitHub Actions Inactive

mateuszpn temporarily deployed to WindowsCILock July 8, 2025 12:08 — with GitHub Actions Inactive

mateuszpn temporarily deployed to WindowsCILock July 8, 2025 12:22 — with GitHub Actions Inactive

mateuszpn temporarily deployed to WindowsCILock July 8, 2025 12:43 — with GitHub Actions Inactive

mateuszpn added 2 commits July 8, 2025 12:47

apply comments

deb9aaf

Signed-off-by: Mateusz P. Nowak <mateusz.p.nowak@intel.com>

add traceable() to Benchmark class

a5b1fd1

Signed-off-by: Mateusz P. Nowak <mateusz.p.nowak@intel.com>

	help="Unitrace tracing for sigle iteration of benchmarks",
	help="Unitrace tracing for single iteration of benchmarks",

		@@ -86,7 +87,14 @@ def get_adapter_full_path():
		), f"could not find adapter file {adapter_path} (and in similar lib paths)"

		def run_bench(

		@@ -0,0 +1,182 @@
		# Copyright (C) 2024-2025 Intel Corporation

		return bench_dir, unitrace_output, unitrace_command


		def handle_unitrace_output(bench_dir, unitrace_output, timestamp):

[UR][Benchmarks] Support for Unitrace for all benchmarks #19320

Are you sure you want to change the base?

[UR][Benchmarks] Support for Unitrace for all benchmarks #19320

Conversation

mateuszpn commented Jul 7, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pbalcer left a comment

Choose a reason for hiding this comment

Uh oh!

mateuszpn commented Jul 8, 2025

Uh oh!

Uh oh!